Add mod-sync: clients can download a host's mod set on mismatch#1961
Add mod-sync: clients can download a host's mod set on mismatch#1961TheMasterCreed wants to merge 24 commits into
Conversation
Rename Filesupport::getRuntimeHash to getLegacyRuntimeHash so the existing combined-SHA-512 stays available for backward-compat on the mod-version handshake wire. Add getPerModHashes and getResourceFolderHashes returning QMap<QString, QByteArray> keyed by mod folder path or resource folder name. hashSingleFolder is a thin delegate over getHash so the 3-path expansion, traversal order, and debug logging are inherited mechanically. No behavior change at existing callers; all three (sendMapInfoUpdate, sendVerifyGameData, readHashInfo) use the renamed combined helper with byte-identical output. Groundwork for a per-mod mismatch report in the lobby/verify dialog.
Replace the legacy combined SHA-512 byte array in MAPINFO and VERIFYGAMEDATA with a qint32 sentinel followed by either a 64-byte SHA-512 (sentinel == LegacyRuntimeHashSize) or two QMap<QString, QByteArray> for engine resources and per-mod content (sentinel == CurrentHashPayloadVersion). Receivers dispatch on the sentinel; unknown values fail closed without consuming further bytes. Old clients reading the new payload read 1 byte as a QByteArray length, fail differentHash, and drop the stream via handleVersionMissmatch without misalignment. New clients reading legacy hosts take the 64-byte branch. readHashInfo now early-outs after GameVersion deserialize on version mismatch, and the new-payload branch populates two intersection-only out-params (mismatchedResourceFolders, mismatchedMods) for the upcoming dialog refactor.
Refactor Multiplayermenu::handleVersionMissmatch from a mutually exclusive else-if chain into an additive multi-section message that can surface several discrepancy categories at once. The dialog now shows up to five sections in diagnostic order: Missing mods (host has, you don't) Extra mods (you have, host doesn't) Version mismatch (mod.txt) Content mismatch Engine resources differ The membership/version sections come from comparing the host and local mod lists; the content and engine-resource sections come from slice 2's intersection-only mismatch out-params. Each section truncates at five items with a "...and N more" tail; the full list is always emitted at GameConsole::eINFO before truncation so console.log captures the complete picture. Game-version mismatch suppresses every other section because readHashInfo's early-out leaves mods, sameMods, and differentHash at their caller defaults; rendering them would mislead. Legacy 64-byte hash payloads and unknown-sentinel fail-closed paths produce no structured detail and fall back to the existing terse pre-PR strings. The unreachable "no detail and not differentHash" branch logs at eERROR for diagnostic purposes if it ever fires. Caller signatures (verifyGameData, clientMapInfo) updated to declare and pass the two new QStringList& out-params.
The function took its mod and version lists by value, so the cosmetic removal loop only mutated locals and the cosmeticModsAllowed game rule was silently a no-op for years. All four call sites in the multiplayer handshake (multiplayermenu.cpp:738, :1188, :2142 and gamemenue.cpp:700) expected mutation. Change the signature to take QStringList & and drop the Q_INVOKABLE attribute (no JS callers; QJSEngine doesn't translate out-params well anyway). Behavior change: cosmetic-only mod content differences will now actually pass the mod-version handshake when cosmeticModsAllowed=ON. This is the documented intent of that setting; players who relied on the broken blocking will be surprised. Slice 4 is also load-bearing for the intersection-only mismatch lists in the named-mod-mismatch dialog, which would otherwise still surface cosmetic noise. Also remove the dead Multiplayermenu::filterCosmeticMods declaration in multiplayermenu.h, which had no definition or callers and was adjacent enough to confuse readers about which helper to call.
Bump Filesupport::CurrentHashPayloadVersion to 2 and add a quint32 capabilities field plus a CapabilityModSync bit. Senders downgrade to v1 wire format when no caps are advertised so existing clients on the parent branch can still join, and the receiver parses all three sentinels (legacy, v1, v2) and surfaces host capabilities through a new out-parameter on readHashInfo. Filesupport::validateModPath now rejects NTFS-illegal characters (less-than, greater-than, colon, double-quote, pipe, question mark, asterisk), trailing dot or space, and Windows reserved device basenames including CONIN$ and CONOUT$, and takes a maxLen parameter for slice-2 relpath callers. Settings registers ModSyncEnabled defaulted false (operator opt-in) plus four caps under [Network]; m_modSyncMaxRelativePathLength is the inside-package path cap and modPath identifiers use Filesupport::ModPathDefaultMaxLen. NetworkCommands gains REQUESTMODSYNC, MODSYNCDATA, MODSYNCREJECT, and MODSYNCCOMPLETE constants and an append-only ModSyncRejectReason enum carried as qint32 on the wire, with ModSyncNoReason=0 reserved so a truthy check on the reason field never falsely reports a reject. Pre-existing fix folded in: Settings::setActiveMods now clears m_activeModVersions before rebuilding. The cmdline --mods flag at commandlineparser.cpp:255 and 260 calls setActiveMods after loadSettings has already populated the version list, so every binary run with --mods previously doubled it. Groundwork for the mod-sync wire surface. The receiver, disk staging, restart-and-rejoin flow, and UI button arrive in later slices.
Introduce ModSyncCaps and ModSyncPackage structs for cap parameters and packaging results. Filesupport::validateRelativeFilePath is the segment-walk equivalent of validateModPath for inside-package relpaths, allowing nested directories while applying the same NTFS-illegal-char and reserved-device rules per path segment. buildModSyncPackage walks a mod folder, filters .git, .svn, __pycache__, .sync-staging-*, and .bak-* directory segments, validates relpaths, enforces per-mod size and file-count caps, and qCompresses a QMap from relPath to file contents. extractModSyncPackage qUncompresses, revalidates caps and paths, and defends against decompression bombs and forged size headers. stageModSync writes the validated QMap to mods/<name>.sync-staging-<pid>/ and deletes the partial staging on any write failure. reapModSyncFolders is the boot-time cleanup, dropping stale staging directories (mtime older than one hour) and pruning per-mod backups beyond a keep-most-recent-N count (default 3). writePendingModSyncManifest and executePendingModSyncManifest implement the JSON-based pending-swap protocol; the manifest lists {staging, final} pairs, the executor renames finalAbs to .bak-<iso> if it already exists then renames staging into place, and the manifest is deleted after successful application.
Reject-reason values are qint32 rather than the NetworkCommands enum so coreengine stays decoupled from networkcommands.h. The kModSync* anonymous-namespace constants mirror NetworkCommands::ModSyncRejectReason and are pinned via static_assert in the receiver translation unit in slice 4.
Groundwork for boot-time apply and per-client request handling. This slice ships filesystem helpers only; no network handlers or boot wiring yet.
extractModSyncPackage now gates the qCompress framing-size header before calling qUncompress, so a forged size cannot trigger pre-allocation past caps.perModBytes. The map decoder is bounded across the whole entry; count is checked against fileCountMax up front, then each entry's length is read separately and validated before any allocation, with keys carried as UTF-8 QByteArrays so the bounded reader does not have to deal with QString's UTF-16BE shape. Duplicate-key rejection was added at the same site. buildModSyncPackage caps serialized size before the qint32 cast on declaredUncompressedSize, in addition to the existing raw and compressed checks, preventing silent truncation if perModBytes is ever raised past 2GB. The cruft filter is now segment-based, with .sync-staging-* and .bak-* matched only on directory segments so a legitimate filename like scripts/foo.bak-test.js is preserved; .DS_Store, Thumbs.db, and desktop.ini are also filtered. stageModSync returns the staging path relative to installRoot so the caller writes the same shape into the manifest that the executor reads, takes ModSyncCaps explicitly, and re-validates relpaths, file count, per-file size, and running total before any disk write. executePendingModSyncManifest validates the manifest staging-path shape (finalRel + ".sync-staging-" + digits) before any rename, requires both staging and final to be directories, rolls the backup back to final on partial-rename failure so the active mod folder is never left missing, and returns the list of applied final paths so callers can distinguish ground truth from the planned swap set. The backup name uses a UTC iso timestamp with millisecond precision plus a numeric counter loop to avoid same-instant collisions, and the manifest size is capped at 1MB before parse to defend against runaway JSON allocation. Defensive negative-cap reject was added at extractor entry. Groundwork for slice 3's boot-time apply path. The manifest format and caller contract are now stable.
Settings::loadSettings now calls Filesupport::executePendingModSyncManifest(m_userPath, m_userPath) followed by Filesupport::reapModSyncFolders(m_userPath) between setUserPath(m_userPath) and setActiveMods(m_activeMods). Ordering matters: the executor must commit any pending swap before setActiveMods runs its existence-check prune, otherwise a just-synced folder would be silently dropped from m_activeMods. m_userPath has already been re-read from ini and normalized through setUserPath at this point, so the executor sees the configured user path; the empty (CWD-relative default install) and non-empty (custom user path) cases both work symmetrically through joinPath. reapModSyncFolders tightened its directory-name matchers so the reaper only acts on the exact slice-2-generated shapes. Staging requires <modPath>.sync-staging-<digits> where the prefix passes validateModPath and the suffix is an all-digit pid. Backup requires <modPath>.bak-<isoStamp> with optional collision counter, where isoStamp is the exact 19-character yyyyMMdd-HHmmsszzzZ format and the optional counter is a hyphen followed by at least one digit. A legitimately weirdly-named mod folder no longer matches. Index-based shape checks were chosen over QRegularExpression to avoid the regex compile cost; the format is captured in a comment at the call site. Groundwork for the slice 4 receiver, which writes the manifest that writePendingModSyncManifest reads here. Slice 4 hand-offs: the receiver translation unit pins kModSync* against NetworkCommands::ModSync* via static_assert once both headers are visible; the manifest writer must use Settings::getInstance()->getUserPath() so the path matches what the executor reads at boot; and any settings mutator that runs after a partial executor failure must consume the executor's applied-path list as ground truth rather than treating the planned swap set as success.
The eDEBUG CONSOLE_PRINT at this call site runs before GameConsole::setLogLevel(m_defaultLogLevel) executes lower in loadSettings, so the line is unreachable at any LogLevel value. Removing it is the cheapest fix; the reaper and executor still log their own actions when they trigger, so the boot path stays diagnosable once LogLevel rises later in the same loadSettings call.
Multiplayermenu::requestModSync sends REQUESTMODSYNC to the host via socketID=0, matching the existing client-originated convention used by startCountdown and sendInitUpdate, and arms a session that tracks staged mods, a pending requested-set, compressed and declared-uncompressed byte counters, and the post-sync active mod list. An empty modsToDownload short-circuits to a settings-only path that calls Settings::stageActiveModsForRestart and skips both network and manifest. handleModSyncRequest validates the protocol version, parses the requested-mod list with bounded readers and a 1024-entry cap, dedupes via QSet at parse time, applies the cosmetic-mod filter against the active map's GameRules, resolves each mod's actual on-disk root through VirtualPaths::find(mod, false) so userPath, CWD, and applicationDirPath layouts all work, enforces totalCap on both compressed wire bytes and declared uncompressed bytes, and streams MODSYNCDATA frames followed by MODSYNCCOMPLETE. handleModSyncData validates the frame's protocolVersion, reads modPath, declaredSize, fileCount, and the compressedBlob through bounded readers that reject declared sizes above qint32 max before any QByteArray::resize call, uses fileCount as both a pre-extract cap gate and a post-extract files.size() cross-check, and removes modPath from the requested-set after a successful stage so duplicate or unrequested deliveries abort the session. handleModSyncComplete requires the requested-set to be empty before applying, calls Settings::stageActiveModsForRestart to write Mods/Mods directly to the ini without perturbing live m_activeMods or m_activeModVersions, then writes the pending-swap manifest as the commit point; if writePendingModSyncManifest fails, Settings::restoreActiveModsRaw rolls the ini back to its prior raw value before cancelModSyncSession tears down the staging directories. Settings::stageActiveModsForRestart returns the prior raw Mods/Mods value and gates on slave/aiSlave/m_updateStep matching saveSettings; restoreActiveModsRaw mirrors the same gate so the rollback path is symmetric. Reject codes are pinned to wire literals via static_assert against NetworkCommands::ModSync* values 0..6. Bounded readers reject declared sizes above 0x7FFFFFFFu and negative caps before any resize, closing the cast-negative path through QByteArray::resize(qint32). Groundwork for the slice 4b UI button (Apply host's mod set), the slice 4c auto-restart and rejoin flow that closes the saveSettings-revert window between MODSYNCCOMPLETE and the actual restart, and the slice 4d settings checkbox for ModSyncEnabled.
Multiplayermenu::handleVersionMissmatch now offers a two-button DialogMessageBox ("Apply host's mod set" and "Leave game") when the host advertised Filesupport::CapabilityModSync, no engine resource folder is drift-mismatched, and either the download list or extras-only list is non-empty. Single-button "Leaving the game again." stays for non-fixable cases (engine resource drift, host without the capability, version-mismatch where readHashInfo early-returned). The dialog text gets a casual closing line "Want me to download host's mod set and apply it on the next start?" when the offer is presented. The handler now collects a parallel modsToDownloadPaths list of raw modPath identifiers alongside the existing display strings (missingHere, versionDiffs, contentDiffs), deduped between version-diff and content-diff branches via QStringList::contains, and computes postSyncActiveMods from the host's filtered mods list plus any locally active cosmetic mod that was filtered out of the comparison when cosmeticAllowed is true. Both recieveData call sites (verifyGameData and clientMapInfo branches) and readHashInfo plumb the new bool cosmeticAllowed out-parameter through from the existing wire-level filter byte so the offer logic does not have to recompute it from the current map's GameRules.
DialogModSyncProgress (objects/dialogs/dialogmodsyncprogress.{h,cpp}) is a new full-stage Box9 dialog with a manual ColorRectSprite progress bar (480 by 24, gray background, green fill), a centered header ("Downloading host's mod set"), a centered detail label ("X / Y mods (N KB)"), and a Cancel button. setProgress recomputes the fill width using float division so the qint32-as-float bug at progressinfobar.cpp:38-49 is sidestepped, and the detail line is recentered each tick. confirmModSync calls requestModSync first and gates on its new bool return: empty download list returns success or failure based on the bool, non-empty creates the progress dialog only after the request was accepted. The cancel lambda inside the progress dialog has its own m_modSyncActive early-return so a queued cancel firing after success or after a previous cancel is a no-op. handleModSyncData calls onModSyncProgress after each successful stage so the dialog updates every frame, and uses the declared uncompressed bytes counter (m_modSyncReceivedUncompressedBytes) for the KB display so the on-disk delta matches what the user sees, not the wire-compressed size. handleModSyncReject and handleModSyncComplete now early-return on !m_modSyncActive at the very top of the function, before any stream reads, matching handleModSyncData's pattern, so a stale or unsolicited frame after cancel or success cannot stack a second failure dialog. cancelModSyncSession tears down the progress dialog ahead of its active-session guard so a request that bailed before arming still cleans up its UI. onModSyncFailed runs reason.toHtmlEscaped() on the host-supplied reject message before interpolating into DialogMessageBox::setHtmlText so a malicious or buggy host cannot inject HTML into the failure dialog.
requestModSync now returns bool: false on already-in-flight, null or server-side network interface, or count cap exceeded; true after a successful settings-only stage or after arming the session and sending REQUESTMODSYNC. The dead m_modSyncTotalRequested member is gone from the header and all reset sites; the dialog's own m_totalMods constructor argument is the single source of truth for "X of Y" displays.
Filesupport::buildModSyncPackage previously called QDir::relativeFilePath on the iterator's emitted file path with modRoot as the base. When installRoot resolved to "." (the default with empty UserPath in ini), modRoot ended up as "./mods/<name>" and the iterator emitted "./mods/<name>/<file>" relative paths to match. QDir::relativeFilePath short-circuits to verbatim when either operand is relative, so the package keys became "mods/<name>/<file>" instead of just "<file>". The client then joined those keys against the staging directory and produced "<staging>/mods/<name>/<file>", a doubly-nested layout where the actual mod files ended up two levels too deep. After the executor renamed staging into place, the post-restart mod folder looked like "mods/<name>/mods/<name>/<file>" and the engine could neither read mod.txt at the expected location nor get a clean per-mod hash, surfacing as a perpetual version mismatch with placeholder version "1.0.0" on every restart. Cache the absolute mod directory once via QDir::absolutePath, then run relativeFilePath against absolute file paths produced by QFileInfo::absoluteFilePath. With both sides absolute, the short-circuit never trips and keys come out as plain file-relative paths regardless of whether installRoot was passed as "" or "." or a normalized absolute UserPath.
handleModSyncRequest's reject path that fires after buildModSyncPackage now translates pkg.rejectReason into a mod-named, reason-specific message before sending it to the client. The previous "Failed to build mod package." string was identical for every reason code, so a user who hit the per-mod size cap on a 700MB audio mod saw the same dialog as someone whose host advertised a path the package builder couldn't read. The new mapping covers ModSyncSizeCapExceeded, ModSyncFileCountCapExceeded, ModSyncInvalidPath, and ModSyncUnknownMod, with a fallback for ModSyncInternalError. The total-cap reject message also picks up the host attribution. Reject codes themselves are unchanged on the wire; this commit only changes the human-readable string the client shows in onModSyncFailed.
handleVersionMissmatch's missingHere branch was queueing a download for any mod the host had active that the client did not have in its active list, even when the mod folder was already on disk under mods/<name>. The downstream effect was an unnecessary REQUESTMODSYNC for mods that just needed activation, which on the host could fall over against the per-mod size cap for large local-only assets like audio packs that both sides already have. After "Apply host's mod set" the client would see a "Mod %1 exceeds the per-mod size cap on the host." dialog and abort the whole session, even though no actual download was needed. Add a VirtualPaths::find probe before adding the mod to modsToDownloadPaths in the j < 0 branch. If the resolver returns a path and that path exists on disk, leave the mod off the download list; the post-sync active list still contains it (because postSyncActiveMods seeds from the host's filtered mods), so the next-boot setActiveMods pass picks it up from disk and enables it. The version-mismatch and content-mismatch branches still queue downloads unconditionally because the local copy in those cases either has the wrong mod.txt version string or the wrong .js/.csv content; both genuinely need replacement.
The on-disk skip path added in 5fa654e29 trusted folder existence alone, which is unsafe: an inactive stale or unrelated folder under the same name would let the settings-only fast path activate the wrong content, the next handshake would surface the same content mismatch, and the user would loop. Plumb the host's per-mod hash map out of readHashInfo (one assignment inside the existing compareMaps lambda, no recomputation, no wire change) so handleVersionMissmatch can verify before skipping. The j<0 missing-from-active branch now reads VirtualPaths::find for the local folder, computes Filesupport::getPerModHashes for that single mod, reads Settings::getModVersion for its mod.txt version, and skips the download only when the local hash equals hostModHashes.value(mod) and the local version equals versions[i]. Empty local hash, empty host hash entry, hash mismatch, or version mismatch all queue the download instead, with an eDEBUG breadcrumb identifying which condition failed. Both readHashInfo call sites (verifyGameData branch and clientMapInfo branch) declare the local QMap and thread it through. The offer predicate that decides whether to surface "Apply host's mod set" was based on modsToDownloadPaths, but that list is now derived (filtered by the disk-presence-and-content gate above), not the canonical "is there anything to fix" signal. With every missing mod successfully verified locally, the download list is empty and the old predicate suppressed the dialog even though settings-only activation was the correct action. Switch the predicate to fire whenever any mismatch class is non-empty: missingHere, versionDiffs, contentDiffs, or extraHere. The unreachable differentHash-with-no-classifications branch keeps all four lists empty, so the predicate stays false and falls through to the existing single-button "Cannot join" dialog without a spurious offer. The all-on-disk-and-content-matched scenario now correctly fires the offer, the empty download list short-circuits requestModSync to the settings-only stage, and the success dialog renders.
stageActiveModsForRestart writes Mods/Mods to QSettings but left m_activeMods untouched, on the original theory that the in-memory list should not change until the user restarts and the next-boot setActiveMods pass rebuilds it. That left a window where any saveSettings call between MODSYNCCOMPLETE and the close-and-relaunch step would overwrite the staged ini with getConfigString(m_activeMods), where m_activeMods still holds the pre-sync stale list. The repro was multiple resync attempts in a row, every restart presenting the same mismatch dialog with the same mods, with disk inspection confirming Mods/Mods reverted to the pre-sync client list rather than the host list it was staged to. saveSettings runs on a number of paths the user can hit between MODSYNCCOMPLETE and the relaunch, including game-quit cleanup and any path that touches a Q_INVOKABLE writer. Assign m_activeMods to the post-sync active list at the same point stageActiveModsForRestart writes the ini, and clear m_activeModVersions so the parallel-list invariant from setActiveMods is preserved. Versions get rebuilt from each mod's mod.txt at next-boot setActiveMods, the same way they would on a clean fresh boot. Mirror the rollback in restoreActiveModsRaw: on manifest-write failure, the prior raw value is written back to QSettings, m_activeMods is rebuilt by splitting that raw value on the comma separator, and m_activeModVersions is cleared the same way. Empty raw value yields an empty list rather than a one-element list containing the empty string, matching getConfigString's round-trip contract. The slave and ai-slave guards already at the top of both functions still short-circuit, so headless processes do not touch their in-memory state.
The DialogMessageBox two-button layout assigns a fixed width per button and the previous "Apply host's mod set" label was truncated to "Apply hos..." at runtime. Shorten to "Apply" so the label fits any reasonable font scale and locale rendering of the same DialogMessageBox button width.
handleModSyncComplete defers onModSyncSucceeded by QTimer::singleShot(500ms) so the progress dialog paints at 100% before teardown. onModSyncSucceeded writes userData/.rejoin.json (host, port, timestamp; no password), sets Mainapp::setRestartArgv with --userPath when the parent had it on the command line and --rejoin-password (plaintext) only on manifest-write success, and calls QCoreApplication::exit(1). main.cpp now passes Mainapp::getRestartArgv to the existing QProcess::startDetached restart path. CommandLineParser::parseArgsPhaseOne consumes --rejoin-password into Mainapp::m_rejoinPassword. WorkerThread::showMainwindow consumes the manifest unconditionally so a slave relaunch still discards it, moves the password from the static slot into a local, and if the manifest is fresh (within 5 minutes) and the launch is not slave or aiSlave, constructs a Multiplayermenu directly with the connected host endpoint and skips the main menu. Multiplayermenu caches the constructor primary endpoint as fallback when the interface has no recorded connected address. Filesupport gains writeRejoinManifest, consumeRejoinManifest, rejoinManifestPath: QSaveFile-atomic write, 4KB read cap, version plus host plus port validated, consumed and deleted on first read regardless of validity.
…rust prompt Settings gains m_modSyncEnabled and m_modSyncKeepBackups checkboxes in the Network options menu. The host toggle was previously INI-only and gates whether CapabilityModSync is advertised in the handshake. The new keep-backups toggle defaults false; when off, executePendingModSyncManifest still creates the .bak directory during the staging swap so a partial-rename failure can roll back, then removes it after the swap succeeds. The boot reaper still runs on the next launch and prunes anything left behind from interrupted sessions. confirmModSync now shows a trust dialog before any host content is downloaded. Clicking Apply on the version mismatch dialog leads to "You are about to install mods from this host. These are unverified scripts that will run in your game. Only continue if you trust this host." with Yes install or Cancel. The settings only branch (no downloads, just toggling activation flags) skips the prompt because no host content is being executed. The download path moves to a new startModSyncDownload helper so confirmModSync stays a thin trust gate and router. resources/ui/options/optionnetworkmenu.xml gets the two label plus checkbox pairs at the bottom of the Network panel. m_modSyncKeepBackups gets a new Q_INVOKABLE getter and setter on Settings, plus the matching Network/ModSyncKeepBackups INI entry next to the other ModSync caps.
DialogModSyncProgress now shows a smoothed download rate, an ETA, and a byte-based bar fraction. Network throughput uses an exponential moving average over compressed-byte deltas with a 50 ms minimum sample window, and ETA divides the remaining uncompressed bytes by a parallel uncompressed-rate EMA so the units match what the bar fills against. When the host's manifest has not arrived the bar falls back to staged mods over total mods. A monotonic floor on the displayed fraction prevents late or out-of-order events from rewinding the visual. The initial join "Connecting" dialog is now held as a Multiplayermenu member so the mod-sync flow can dismiss it before opening the progress dialog, eliminating a stacked second Cancel button. DialogConnecting::connected and ::cancel stop both their internal timers so a retained smart pointer cannot fire a late connectionTimeout. A queued lambda on sigConnected then drops the smart pointer after the dialog has detached itself. The host emits a new MODSYNCMANIFEST frame before the first MODSYNCDATA carrying per-mod expected uncompressed sizes. The client sums only entries that match its requested set, feeds the total to the dialog, and refuses any later downward revision. Older hosts skip the frame and clients fall back cleanly; older clients drop the unknown frame on the existing dispatcher branch. The host now prebuilds all packages so the manifest can carry exact sizes, releasing each compressed blob after its MODSYNCDATA send so peak memory stays near one total cap. qCompress level drops from default 6 to 1 in buildModSyncPackage; the size penalty on text-heavy mods is small and the host CPU stall improvement is meaningful for slow-CPU clients. qUncompress is level-agnostic so the wire format is unchanged.
Mod-sync now delivers each mod as a sequence of MODSYNCMODBEGIN, one or more MODSYNCMODCHUNK frames, and MODSYNCMODEND when both endpoints opt in via the new ModSyncClientFlagChunked bit on REQUESTMODSYNC. Chunks are 1 MiB each (declared as a wire contract on both sides; the chunkCount and per-chunk size are validated against this constant). The receiver advances the uncompressed-byte counter proportionally on every chunk and snap-corrects to the exact declared total on MODSYNCMODEND, so the dialog's bar fraction and ETA finally move during a single large mod transfer. Cap enforcement still runs against the live counter every chunk, drip-feed defense intact. The legacy single-frame MODSYNCDATA path is preserved verbatim for older hosts and older clients. Compatibility holds in all four pair combinations: new-on-new speaks chunked, new-on-old falls through to legacy because the trailing clientFlags qint32 is silently ignored by older hosts, old-on-new sends no flag and the host serves legacy, old-on-old continues unchanged. The host send loop is no longer synchronous. handleModSyncRequest pre-builds packages and emits MODSYNCMANIFEST as before, then hands the queue to a member ModSyncSendState and kicks off pumpModSyncSend via QTimer::singleShot. Each tick emits exactly one frame, yields to the event loop, and reschedules itself, so a multi-hundred-megabyte transfer no longer pins the GUI thread. Concurrent REQUESTMODSYNC from a second peer rejects with the new ModSyncBusy reason rather than clobbering the in-flight pump; per-socket send state is the right future hardening. The optional clientFlags trailer parse on REQUESTMODSYNC is strict: zero trailing bytes mean legacy, exactly one qint32 means new client. Partial qint32 (1 to 3 bytes) and post-flag-trailing bytes both reject as malformed instead of being silently treated as flags zero. BEGIN-time validation rejects before any QByteArray reserve. compressedTotal is bounded by perModCap and explicitly capped to the qint32 range as belt-and-suspenders against a future widening of perModBytes. chunkCount must equal ceil(compressedTotal divided by MOD_SYNC_CHUNK_BYTES) and stay under the sanity ceiling. modPath must validate and live in the requested set, the same gate the legacy path uses. CHUNK-time validation enforces sequential chunkIndex, modPath match against the in-flight mod, exact size match against the position in the sequence, and accumulated overflow against compressedTotal. END-time validation re-checks the size and chunk-count totals before extract, then runs the existing extractModSyncPackage and stageModSync path on the assembled blob. State guards reject MODSYNCDATA mixed with chunked, MODSYNCMODBEGIN while another is in flight, and MODSYNCCOMPLETE while a chunked mod is unfinished. cancelModSyncSession now clears the host-side send state ahead of the client-side active-flag guard so a host teardown path that runs with the client flag false still tears down the pump. requestModSync and the success paths reset the per-mod chunk accumulator alongside the existing counters so a cancel-and-reconnect leaves no stale per-mod state behind. The pump itself short-circuits on socketID equal to zero so a reset between ticks is safe. Files: multiplayer/multiplayermenu.cpp, multiplayer/multiplayermenu.h, multiplayer/networkcommands.h.
The detail and header lines on the mod-sync progress dialog shifted to the left of the dialog because the constructor centered each field by computing x from m_*->getTextRect().width() right after setHtmlText. Oxygine's TextField returns an unreliable rect at that point, especially when text length grows substantially in a later setHtmlText call (the chunked-transfer detail line) but also in some configurations on first construction (the header). Both fields now use an explicit full-stage width via setSize and place at x=0; the existing HALIGN_MIDDLE on the TextStyle then centers any text inside the laid-out field on every setHtmlText. setProgress no longer needs to re-position the detail field on each update.
The mismatch dialog asked "Want me to download host's mod set and apply it on the next start?", which leaves the user wondering whether they need to relaunch the game manually. The flow already triggers an automatic restart and rejoin via the rejoin manifest after staging completes, so spell that out: "Want me to download host's mod set, apply it, and restart automatically?".
| return false; | ||
| } | ||
| const QStringList segments = modPath.split(QChar('/')); | ||
| if (segments.size() != 2) |
There was a problem hiding this comment.
No magic number.
Please add a constant telling whhat 2 means and use it instead
| } | ||
| for (const QChar c : segment) | ||
| { | ||
| if (c.unicode() < 0x20 || c.unicode() == 0x7F) |
There was a problem hiding this comment.
Same here use a constant with a telling name for 0x20 and 0x7F
| static const QSet<QString> kReservedNames = { | ||
| QStringLiteral("CON"), QStringLiteral("PRN"), QStringLiteral("AUX"), QStringLiteral("NUL"), | ||
| QStringLiteral("CONIN$"), QStringLiteral("CONOUT$"), | ||
| QStringLiteral("COM0"), QStringLiteral("COM1"), QStringLiteral("COM2"), QStringLiteral("COM3"), |
There was a problem hiding this comment.
Shouldn't these patterns be COM + Number and LPT + number
at least COM-Ports don't stop at 9
| } | ||
| for (const QChar c : seg) | ||
| { | ||
| if (c.unicode() < 0x20 || c.unicode() == 0x7F) |
There was a problem hiding this comment.
Use same constant as in the other function aka make it locally global for this CPP:
| return false; | ||
| } | ||
| } | ||
| static const QSet<QString> kReserved = { |
There was a problem hiding this comment.
This is the same map as in the function above.
Make one constant for this so they don't need to be duplicated.
Also the number problemas above
| } | ||
| Filesupport::writeByteArray(stream, hostHash); | ||
| // Stay on parent v1 wire format when no caps advertised so v1 clients still join. | ||
| if (capabilities == 0) |
There was a problem hiding this comment.
Please define this as legacy variant constant
| const char* const MODSYNCCOMPLETE = "MODSYNCCOMPLETE"; | ||
|
|
||
| // Bit flags appended (optional) to REQUESTMODSYNC; older hosts read past them via QDataStream EOF and treat absent flags as zero. | ||
| enum ModSyncClientFlag : qint32 |
| }; | ||
|
|
||
| // Append-only; serialize as qint32, not the enum's underlying type. 0 reserved so callers can use truthy reads. | ||
| enum ModSyncRejectReason |
|
|
||
| QString DialogModSyncProgress::formatBytes(qint64 bytes) | ||
| { | ||
| if (bytes < 1024) |
There was a problem hiding this comment.
Magic number.
I stop here for now.
| setObjectName("DialogModSyncProgress"); | ||
| #endif | ||
| Interpreter::setCppOwnerShip(this); | ||
| ObjectManager* pObjectManager = ObjectManager::getInstance(); |
There was a problem hiding this comment.
New ui's should use the xml approach instead of hard coded ones
|
Going to come back with smaller changes as I get more familiar with the codebase |
|
Sidenote: |
|
I think it's a great improvement. |
|
Thanks, that means a lot. I closed it a bit hastily because the review surface felt overwhelming and I got in my own head about my C++ depth. Going to be slower than someone fluent in the codebase but I'll get there if you want me to reopen 🙂 |
|
Yes i reopened it. @TheMasterCreed all I did is marking the code lines (might missed some) where I wanted a code improve and documented what that code improve is. Normally you can quote that you fixed the issue I noted and I'll check if everything is fine and once eveything is fine we merge it. Basically it's a small quality gate and during my wokr and writing CoW code i learned things that you shouldn't do and I mark them. |
|
Most of my comments should be easily fixable in a couple of minutes. |
|
Sry if my feedback felt harsh for you. |
|
No need to apologize at all, the feedback wasn't harsh, I just got in my own head. Thanks for the patience and for explaining the workflow. I'll start working through the comments when I have the time 😁 |
Adds an optional flow that lets a connecting client whose mod set differs from the host download the host's mods, stage them to disk, restart, and auto-rejoin the lobby.
The feature is off by default. The host enables it under Network options ("Allow clients to sync mods from this host:"); the client signs off on each individual sync via an explicit trust dialog before any host content is downloaded. There is no per-host trust memory, every sync requires a fresh user click.
Wire format is a small additive set of QDataStream-over-TCP frames gated by capability and flag bits, with full back-compat to older clients and hosts. Old hosts ignore unknown frames; old clients see no-op fallback behavior. Per-mod and per-total size caps, per-mod file-count caps, and relpath length caps are all configurable from Settings with conservative defaults.
Protocol surface in
multiplayer/networkcommands.handmultiplayer/multiplayermenu.{h,cpp}. New frames:REQUESTMODSYNC,MODSYNCMANIFEST,MODSYNCDATA,MODSYNCMODBEGIN,MODSYNCMODCHUNK,MODSYNCMODEND,MODSYNCREJECT,MODSYNCCOMPLETE. Per-frame protocol-version field, append-only reject-reason enum,Filesupport::CapabilityModSyncbit on the existing handshake, optionalModSyncClientFlagChunkedbit onREQUESTMODSYNC.Filesystem helpers in
coreengine/filesupport.{h,cpp}build aqCompress'd package per mod (level 1 to limit host CPU stall on text-heavy mods), validate relpaths and caps defense-in-depth on both endpoints, stage each received mod intomods/<name>.sync-staging-<pid>/, and write a pending-manifest JSON that a boot-time executor renames into place atomically. A reaper trims orphaned.bak-<iso>directories on subsequent launches.Settings additions in
coreengine/settings.{h,cpp}andresources/ui/options/optionnetworkmenu.xml:ModSyncEnabled(host opt-in),ModSyncMaxPerModBytes,ModSyncMaxTotalBytes,ModSyncMaxFiles,ModSyncMaxRelativePathLength,ModSyncKeepBackups. Two of these surface as checkboxes in the Network options panel; the rest are admin-tunable via the INI.UI in
objects/dialogs/dialogmodsyncprogress.{h,cpp}and a refactor ofobjects/dialogs/dialogconnecting.cpp. Mismatch dialog gains an "Apply" button; trust acknowledgment dialog ("You are about to install unverified mods from this host...") gates downloads. Progress dialog shows a smoothed download rate, ETA, and a byte-based bar that advances during a single in-flight mod via the chunked-transfer path. Fixes a regression where the initial join "Connecting" dialog stacked a duplicate Cancel button alongside the progress dialog.After staging,
Mainapp::setRestartArgvqueues a--rejoin-password=argv plus a.rejoin.jsonmanifest, the game restarts viaQCoreApplication::exit(1), and the next launch rejoins the original lobby host without the user having to retype the password or browse back.Tested
Verified end-to-end across two machines (LAN and over the open internet at ~500 to 900 KB/s) with a real 800 MB mod set:
mods/<name>.sync-staging-<pid>/directories, no orphaned state.MODSYNCDATApath.REQUESTMODSYNCarriving while a transfer is in flight gets aModSyncBusyreject with a clear failure dialog rather than clobbering the in-flight pump.ModSyncKeepBackups=truethe previous mod folders persist as.bak-<iso>; with the defaultfalsethey are removed after the swap succeeds, with the reaper handling anything left behind.Notes
The feature is opt-in per host. No existing flow is altered when
ModSyncEnabled=false; the capability bit is simply not advertised and clients see the existing leave-the-game behavior on mismatch. Mod-sync downloads are bounded by both per-mod and per-total caps; a hostile or buggy host cannot cause the client to allocate or stage past the configured limits, and mid-stream cap violations abort the session.Future hardening deferred from this PR: per-mod SHA-256 integrity check in the manifest, per-socket pump state instead of busy-reject for concurrent peers, host-side cancel hook on client disconnect, and resource-folder sync (currently a
resourceDriftmismatch still blocks Apply).